Reintroduce GitHub PAT setup and devcontainer auth#6
Conversation
![jinn-agent[bot]](https://avatars.githubusercontent.com/u/26201488?s=60&v=4){kind=small}
There was a problem hiding this comment.
The PR correctly reintroduces GitHub PAT setup functionality with good security practices. The code includes proper token validation, warning system for broad-access tokens, and secure credential handling. However, there are a few security and reliability concerns that should be addressed.
…e, atomic sed - Return early in setup_gh_auth() when GH_TOKEN is invalid, skipping the useless gh auth setup-git call - Replace sed -i.bak with grep -v + mktemp/mv to avoid .bak file leaks - Store tokens in macOS Keychain instead of plaintext in shell profile; fall back to plaintext on Linux with a warning - Add source guard to setup-gh-token.sh for testability - Add pytest tests for _warn_broad_token() and setup_gh_auth() - Add bash tests for remove_existing_gh_token() and store_token() Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
This PR reintroduces GitHub PAT setup functionality with comprehensive documentation and safety features. The implementation includes secure token storage on macOS (Keychain), token validation, and appropriate warnings for broad-access tokens. No high-confidence security issues found - the code follows security best practices including input validation, error handling, and proper credential management.
- Log stderr from gh auth status on failure for debuggability - Replace grep/head/cut chain with jq in curl fallback for robust JSON parsing in validate_token() - chmod 600 shell profile after writing plaintext token on Linux - Add test for stderr logging and chmod 600 behavior Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
This PR reintroduces GitHub authentication features with solid security practices. The token validation, secure storage on macOS Keychain, and fine-grained PAT warnings are well-implemented. Key strengths include proper error handling in setup_gh_auth(), comprehensive test coverage, and secure token management patterns. No high-confidence security or correctness issues identified.
- Replace sed -i.bak with grep -v + mktemp + mv (no backup files) - Add mv error handling to clean up tmpfile on failure - Replace grep/head/cut with jq for JSON parsing in curl fallback - Add chmod 600 for Linux plaintext token fallback - Add early return on invalid/expired GH_TOKEN in post_install.py - Add Keychain helpers, --remove flag, and write_profile_export() Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
If mv fails, clean up the tmpfile and return non-zero instead of leaving a dangling temp file and continuing with a missing profile. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add audit-contract and web3-diff-review slash commands
- Add PostToolUse hook warning on high-risk Solidity patterns (delegatecall,
selfdestruct, assembly, raw call, mint/burn, upgrade, initialize)
- Deny broadcasting forge/cast commands that take --private-key
- Remove l1-auditor from trailofbits/config persona list
- Fix devcontainer Dockerfile path (build context is repo root)
2 participants
Summary
GH_TOKENpassthrough in devcontainer.jsonsetup_gh_auth()and_warn_broad_token()to post_install.pyscripts/setup-gh-token.shfor macOS Keychain PAT managementclaude-containerCLI docs and GitHub Authentication sectionContext
PR #4 was merged then reverted (#5) due to ordering issues with PRs #2 and #3. Those PRs have since landed and the ordering is correct. This reintroduces #4's changes with merge conflicts resolved cleanly.
🤖 Generated with Claude Code